Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[VL] Customize VCPKG build features according to user's build options #7052

Merged
merged 5 commits into from
Sep 23, 2024

Conversation

PHILO-HE
Copy link
Contributor

@PHILO-HE PHILO-HE commented Aug 28, 2024

What changes were proposed in this pull request?

When vcpkg is enabled, installing native dependencies related to test, hdfs, s3, gcs and abfs should be optional, which depends on user-specified build options. We can put always-required dependencies as vcpkg default feature and enable other feature according to build option.

How was this patch tested?

CI.

@github-actions github-actions bot added the BUILD label Aug 28, 2024
Copy link

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/apache/incubator-gluten/issues

Then could you also rename commit message and pull request title in the following format?

[GLUTEN-${ISSUES_ID}][COMPONENT]feat/fix: ${detailed message}

See also:

@PHILO-HE PHILO-HE force-pushed the control-vcpkg-features branch from 749c594 to a938b8f Compare August 28, 2024 10:00
@PHILO-HE PHILO-HE force-pushed the control-vcpkg-features branch from 0d8a702 to 8006034 Compare September 20, 2024 11:58
@PHILO-HE PHILO-HE requested a review from rui-mo September 23, 2024 01:13
@PHILO-HE PHILO-HE merged commit cc2be36 into apache:main Sep 23, 2024
43 checks passed
export VCPKG_TRIPLET_INSTALL_DIR=${SCRIPT_ROOT}/vcpkg_installed/${VCPKG_TRIPLET}
export EXPORT_TOOLS_PATH="${VCPKG_TRIPLET_INSTALL_DIR}/tools/protobuf"

if [ "\${GLUTEN_VCPKG_ENABLED:-}" != "${VCPKG_ROOT}" ]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"\${GLUTEN_VCPKG_ENABLED:-}" or "${GLUTEN_VCPKG_ENABLED:-}"?

export VCPKG_TRIPLET=${VCPKG_TRIPLET}

export CMAKE_TOOLCHAIN_FILE=${SCRIPT_ROOT}/toolchain.cmake
export PKG_CONFIG_PATH=${VCPKG_TRIPLET_INSTALL_DIR}/lib/pkgconfig:${VCPKG_TRIPLET_INSTALL_DIR}/share/pkgconfig:\${PKG_CONFIG_PATH:-}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\${PKG_CONFIG_PATH:-} vs ${PKG_CONFIG_PATH:-}?

Comment on lines +10 to 11
init_vcpkg_env=$("${SCRIPT_ROOT}/init.sh" $@)
eval "$init_vcpkg_env"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need this eval?

@zhztheplayer
Copy link
Member

@PHILO-HE

The change breaks one of my daily benchmark jobs. Relevant log:

https://gist.github.com/zhztheplayer/f3f4d946d42060694c743f32ab38ba7b

zhztheplayer added a commit to zhztheplayer/gluten that referenced this pull request Sep 23, 2024
@zhztheplayer
Copy link
Member

@PHILO-HE I am testing a fix #7315

zhztheplayer added a commit that referenced this pull request Sep 24, 2024
sharkdtu pushed a commit to sharkdtu/gluten that referenced this pull request Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants